Skip to content

feat(platform): multi-team documents and preview sidebar#755

Merged
Israeltheminer merged 9 commits into
mainfrom
feat/documents-table-ui-polish
Mar 11, 2026
Merged

feat(platform): multi-team documents and preview sidebar#755
Israeltheminer merged 9 commits into
mainfrom
feat/documents-table-ui-polish

Conversation

@Israeltheminer
Copy link
Copy Markdown
Collaborator

@Israeltheminer Israeltheminer commented Mar 11, 2026

Summary

  • Migrate document team assignment from single teamId to multi-team teamIds array across backend and frontend
  • Redesign team tags dialog with checkbox-based multi-select, empty state with settings link, and organization-wide toggle
  • Add metadata details sidebar to document preview dialog showing file info, source, RAG status, teams, and upload details
  • Update Convex queries and mutations for multi-team access control and document filtering

Test plan

  • Verify documents can be assigned to multiple teams via the team tags dialog
  • Test removing all teams reverts document to organization-wide access
  • Check document preview dialog shows details sidebar with correct metadata
  • Verify team column in documents table shows multiple team names with overflow indicator
  • Test backend access control respects multi-team membership
  • Run document component tests (team tags dialog, preview image, preview routing)

Summary by CodeRabbit

Release Notes

  • New Features

    • Image preview support with zoom controls for document images.
    • Multiple team assignment for documents with redesigned team management dialog.
    • New filters for documents: RAG status and source.
    • Document metadata sidebar in preview showing size, source, RAG status, teams, uploader, and modification date.
  • Improvements

    • Filter button redesigned with visible label and updated icon.
    • Document table columns improved with better formatting and display options.
    • Terminology updates for consistency (e.g., "Modified" → "Updated").

- Add RAG status, source, and teams filter popover to documents table
- Fix delete dialog to match Pencil design (bold filename, simplified copy)
- Use abbreviated month format (ll LT) in date columns with timezone
- Add customFormat prop to CopyableTimestamp component
- Show source column as text labels instead of icons
- Left-align size column
- Fix filter button to show icon + "Filter" text label
- Fix i18n interpolation bug ({{count}} → {count}) in nSelected translation
- Remove uppercase from filter section titles
- Update selected count badge to neutral gray style
- Clean up design comments files
- Fix filter button aria-label to match visible "Filter" text
- Use single i18n key with {name} placeholder for delete confirmation
- Append timezone abbreviation to OneDrive file table date column
- Add clarifying comment for preset="long" in CopyableTimestamp usage
- Add timezoneShort to useMemo dependency array in OneDrive file table
- Update filter button test to match new singular aria-label
…m tags dialog

Add multi-team assignment for documents replacing single-team model.
Refactor team tags dialog with checkbox-based selection and empty state.
Add details sidebar to document preview dialog with metadata display.
Update backend to support teamIds array and multi-team access queries.
Copy link
Copy Markdown

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Mar 11, 2026

📝 Walkthrough

Walkthrough

This pull request refactors the documents feature to support multiple team associations per document. Key changes include migrating from single teamId to teamIds array across backend Convex mutations, types, and frontend components; introducing a PreviewPane wrapper component and new DocumentPreviewImage component with zoom functionality; restructuring DocumentPreviewDialog with a sidebar displaying document metadata; updating CopyableTimestamp with optional customFormat prop; refactoring DocumentTeamTagsDialog from single-select to multi-select checkboxes; adding comprehensive test coverage for document preview routing and team tag dialog; updating filter UI components and styling; and expanding translation strings for new labels, filters, and terminology. The design documentation file was removed entirely.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 32.14% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately summarizes the main changes: introducing multi-team document support and adding a preview sidebar to the document preview dialog.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/documents-table-ui-polish

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 12

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
services/platform/convex/documents/mutations.ts (1)

97-97: 🧹 Nitpick | 🔵 Trivial

Note: createDocumentFromUpload still uses single teamId.

This mutation retains the single-team teamId parameter while updateDocument now accepts teamIds[]. This asymmetry may be intentional for the upload flow, but consider whether multi-team assignment at upload time would be useful in the future.

Also applies to: 147-147

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/convex/documents/mutations.ts` at line 97, The comment
points out that createDocumentFromUpload still accepts a single teamId while
updateDocument accepts teamIds[], causing asymmetry; decide whether uploads
should support multiple teams and, if so, change the schema for
createDocumentFromUpload (and the other occurrence around the second spot) from
teamId: v.optional(v.string()) to teamIds: v.optional(v.array(v.string())) and
update any code paths that construct or persist the document (e.g., the
createDocumentFromUpload handler and related helpers) to accept and iterate the
array; if multi-team on upload is intentionally unsupported, instead add a short
inline comment near createDocumentFromUpload explaining that single-team uploads
are deliberate to avoid confusion.
services/platform/convex/documents/update_document.ts (1)

36-48: ⚠️ Potential issue | 🔴 Critical

Validate target teams against the document organization, not just user membership.

getUserTeamIds() is global to the caller, so a user who belongs to teams in multiple organizations can attach this document to a foreign team ID here. Load each target team and reject anything missing or owned by another organization before accepting it. Based on learnings: the repo validates team ownership by loading the team and comparing organizationId before mutating team-related associations.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/convex/documents/update_document.ts` around lines 36 - 48,
The current team validation in update_document.ts only checks membership via
getUserTeamIds and can allow attaching to teams from other organizations; update
the logic to, for each id in args.teamIds, load the team record (e.g., via the
existing team loader function such as getTeamById or equivalent) and verify the
team exists and that team.organizationId === document.organizationId before
accepting it; if any team is missing or has a different organizationId, throw a
descriptive error like "Cannot assign document to a team in a different
organization"; retain the existing user membership check (getUserTeamIds and
userTeamSet) so you enforce both existence/ownership and that the user belongs
to the target teams.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@services/platform/app/components/ui/data-display/copyable-timestamp.tsx`:
- Around line 69-74: The current logic appends timezoneShort to baseFormatted
regardless of customFormat, causing customFormat to be improperly altered;
update the construction of formatted (and its use of baseFormatted/formatDate)
so that timezoneShort is only appended when showTimezone is true AND no
customFormat is provided (i.e., respect customFormat as a full override). Locate
the variables baseFormatted, formatted, formatDate, customFormat, showTimezone,
timezoneShort and dateObj in copyable-timestamp.tsx and change the conditional
that builds formatted to skip post-processing of baseFormatted when customFormat
is present.

In
`@services/platform/app/features/documents/components/__tests__/document-preview-routing.test.tsx`:
- Around line 58-74: The mock implementation of lazyComponent has a race where
the closure variable Component is set asynchronously but the returned component
reads it synchronously, so tests never re-render; update the mock
(lazyComponent) to use React state/effect inside the returned function component
so that when the factory promise resolves it calls setState and triggers a
re-render (i.e., use React.useState/React.useEffect to hold the resolved
component and update it on factory().then(...)), ensure you import React (import
* as React from 'react') in the mocks file, and remove/replace relying on
flushMicrotasks by using Testing Library's waitFor in tests if needed.

In
`@services/platform/app/features/documents/components/document-preview-dialog.tsx`:
- Around line 59-65: The preview currently resolves a single team via the
teamName useMemo (checking doc.teamId) which drops multi-team assignments;
change this to map the document's team array (e.g., doc.teams or doc.teamIds—use
the actual property on the document) to find all matching entries in the teams
list and return a joined label (e.g., comma-separated names), and if that
resulting array is empty fall back to the organization-wide label; update the
same logic where a similar single-team resolution appears (the other occurrence
referenced around lines 114-116) to use the document's team array and the same
fallback behavior.

In
`@services/platform/app/features/documents/components/document-preview-image.tsx`:
- Around line 23-33: The component's isLoading/hasError state is not reset when
the incoming url prop changes, so reuse of the component keeps previous
load/error state; add a useEffect that watches the url variable and resets
setIsLoading(true) and setHasError(false) whenever url changes so each new image
starts in the loading state; keep the existing handleLoad and handleError
callbacks but ensure the effect runs before a new load begins to avoid showing
stale error/success UI.
- Line 5: The import for ZoomPanViewer in document-preview-image.tsx is
incorrect; update the import to match the actual export and file location (use
the correct relative path or named/default export) so knip can resolve it—locate
where ZoomPanViewer is defined/exported, replace the current import line with
the exact module path and export form that matches that definition, and verify
the component builds.

In `@services/platform/app/features/documents/components/document-preview.tsx`:
- Around line 148-150: The IMAGE_EXTENSIONS set currently excludes TIFF/TIF so
files with extension 'tiff' or 'tif' fall through to download-only; update the
IMAGE_EXTENSIONS constant to include 'tiff' and 'tif' (or add a normalized check
for these extensions where extension is derived) so DocumentPreviewImage is
returned for TIFFs, and optionally add a comment or a feature flag to handle
browsers that don't render TIFF by falling back to conversion/preview generation
in the preview pipeline.

In
`@services/platform/app/features/documents/components/document-team-tags-dialog.tsx`:
- Around line 190-239: The group of checkboxes currently rendered with <div
role="group"> should be a native fieldset with a visually hidden legend to
satisfy accessibility linting; replace the outer <div role="group"
aria-label={tDocuments('teamTags.title')}> with a <fieldset> keeping the same
classNames and inner structure, add a visually hidden <legend> containing
tDocuments('teamTags.title'), and remove the role/aria-label from the wrapper;
ensure existing handlers and props (handleSelectOrgWide, handleToggleTeam,
teams, selectedTeamIds, Checkbox, Text) and the button classes/disabled logic
remain unchanged so behavior and styling are preserved.
- Around line 141-161: The footer and the org-wide control are currently
conditionally hidden by hasTeams, preventing submitting teamIds: [] when there
are no teams; update the JSX in document-team-tags-dialog.tsx so the Save/Cancel
footer (the Button group using handleSubmit and handleClose with
isSubmitting/hasChanges) always renders regardless of hasTeams, and ensure the
org-wide control (the control rendered around lines 164-188) is rendered even
when hasTeams is false; keep the team-selection UI conditional on hasTeams but
allow handleSubmit to accept and submit an empty teamIds array when no teams are
present.

In `@services/platform/app/features/documents/components/documents-table.tsx`:
- Around line 192-197: The team filter uses doc.teamId (single value) instead of
checking all assigned teams, so documents with multiple team assignments are
missed; update the filter in the documents-table component to check doc.teamIds
(array) against selectedTeamIds by creating a Set from selectedTeamIds and using
Array.prototype.some (or Array.prototype.every depending on desired semantics)
to test if any of doc.teamIds exist in the set (e.g., teamIdSet.has(id) inside
doc.teamIds.some). Ensure you keep the same filtered variable and only apply
this change inside the existing selectedTeamIds.length > 0 block where
doc.teamId is currently referenced.

In
`@services/platform/app/features/documents/components/onedrive-import/onedrive-file-table.tsx`:
- Around line 47-48: The code is using timezoneShort from useFormatDate() (which
uses new Date()) so the suffix reflects "now" rather than the file's timestamp;
instead compute the timezone suffix from the row's timestamp and use formatDate
with the row's lastModified value so both the human date string and the suffix
are normalized to that specific time. Concretely, replace uses of timezoneShort
and Date(row.lastModified) in the OneDrive file table rendering (references:
useFormatDate(), timezoneShort, formatDate(), lastModified) with a timezone
derived from new Date(row.lastModified) (or call a date-aware helper on that
timestamp) and call formatDate(row.lastModified) everywhere (including the other
occurrences around the same component at the noted locations).

In
`@services/platform/app/features/documents/hooks/use-documents-table-config.tsx`:
- Around line 199-211: The runtime filter(Boolean) on names (computed from
teamIds.map(id => teamMap.get(id))) doesn't narrow (string | undefined)[] to
string[]; update the filter to use a type predicate such as filter((n): n is
string => n !== undefined) so that names becomes string[] for subsequent
slice/join operations and functions/constants like MAX_VISIBLE, names, teamIds,
and teamMap are handled with correct TypeScript typing.

In `@services/platform/convex/documents/validators.ts`:
- Around line 68-69: Change the validator and data flow so teamIds is the
canonical always-present array: replace the optional schema
v.optional(v.array(v.string())) for the teamIds field in validators.ts with a
required v.array(v.string()), update any corresponding validators that reference
teamId/teamIds, and in the document transform function (the place that
normalizes legacy documents) coerce missing/undefined teamIds to an empty array
([]). Also update the TypeScript shape in services/platform/types/documents.ts
to declare teamIds: string[] so the runtime validator, transform, and type
definitions are consistent.

---

Outside diff comments:
In `@services/platform/convex/documents/mutations.ts`:
- Line 97: The comment points out that createDocumentFromUpload still accepts a
single teamId while updateDocument accepts teamIds[], causing asymmetry; decide
whether uploads should support multiple teams and, if so, change the schema for
createDocumentFromUpload (and the other occurrence around the second spot) from
teamId: v.optional(v.string()) to teamIds: v.optional(v.array(v.string())) and
update any code paths that construct or persist the document (e.g., the
createDocumentFromUpload handler and related helpers) to accept and iterate the
array; if multi-team on upload is intentionally unsupported, instead add a short
inline comment near createDocumentFromUpload explaining that single-team uploads
are deliberate to avoid confusion.

In `@services/platform/convex/documents/update_document.ts`:
- Around line 36-48: The current team validation in update_document.ts only
checks membership via getUserTeamIds and can allow attaching to teams from other
organizations; update the logic to, for each id in args.teamIds, load the team
record (e.g., via the existing team loader function such as getTeamById or
equivalent) and verify the team exists and that team.organizationId ===
document.organizationId before accepting it; if any team is missing or has a
different organizationId, throw a descriptive error like "Cannot assign document
to a team in a different organization"; retain the existing user membership
check (getUserTeamIds and userTeamSet) so you enforce both existence/ownership
and that the user belongs to the target teams.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0a0b19f0-3d35-44e0-832e-56c44bbc528c

📥 Commits

Reviewing files that changed from the base of the PR and between 1b37de0 and ee63efa.

⛔ Files ignored due to path filters (1)
  • design/images/comments/knowledge-import-button.png is excluded by !**/*.png
📒 Files selected for processing (34)
  • design/comments.md
  • services/platform/app/components/ui/data-display/copyable-timestamp.tsx
  • services/platform/app/components/ui/data-table/data-table-filters.test.tsx
  • services/platform/app/components/ui/data-table/data-table-filters.tsx
  • services/platform/app/components/ui/dialog/view-dialog.stories.tsx
  • services/platform/app/components/ui/filters/filter-button.tsx
  • services/platform/app/components/ui/filters/filter-section.test.tsx
  • services/platform/app/components/ui/filters/filter-section.tsx
  • services/platform/app/features/documents/components/__tests__/document-preview-image.test.tsx
  • services/platform/app/features/documents/components/__tests__/document-preview-routing.test.tsx
  • services/platform/app/features/documents/components/__tests__/document-team-tags-dialog.test.tsx
  • services/platform/app/features/documents/components/document-delete-dialog.tsx
  • services/platform/app/features/documents/components/document-preview-dialog.tsx
  • services/platform/app/features/documents/components/document-preview-docx.tsx
  • services/platform/app/features/documents/components/document-preview-image.tsx
  • services/platform/app/features/documents/components/document-preview-pdf.tsx
  • services/platform/app/features/documents/components/document-preview-text.tsx
  • services/platform/app/features/documents/components/document-preview-xlsx.tsx
  • services/platform/app/features/documents/components/document-preview.tsx
  • services/platform/app/features/documents/components/document-row-actions.tsx
  • services/platform/app/features/documents/components/document-team-tags-dialog.tsx
  • services/platform/app/features/documents/components/documents-table.tsx
  • services/platform/app/features/documents/components/onedrive-import/onedrive-file-table.tsx
  • services/platform/app/features/documents/components/preview-pane.tsx
  • services/platform/app/features/documents/hooks/use-documents-table-config.tsx
  • services/platform/convex/documents/get_accessible_document_ids.ts
  • services/platform/convex/documents/mutations.ts
  • services/platform/convex/documents/team_fields.ts
  • services/platform/convex/documents/transform_to_document_item.ts
  • services/platform/convex/documents/update_document.ts
  • services/platform/convex/documents/validators.ts
  • services/platform/convex/lib/team_access.ts
  • services/platform/messages/en.json
  • services/platform/types/documents.ts
💤 Files with no reviewable changes (1)
  • design/comments.md

Comment thread services/platform/app/features/documents/components/document-preview-dialog.tsx Outdated
Comment thread services/platform/app/features/documents/components/document-team-tags-dialog.tsx Outdated
Comment on lines +192 to +197
if (selectedTeamIds.length > 0) {
const teamIdSet = new Set(selectedTeamIds);
filtered = filtered.filter(
(doc) => doc.teamId && teamIdSet.has(doc.teamId),
);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify DocumentItem type includes teamIds field
rg -n 'interface DocumentItem|type DocumentItem' --type=ts -A 20 services/platform/types/documents.ts

Repository: tale-project/tale

Length of output: 801


🏁 Script executed:

rg -n 'teamIds' services/platform/app/features/documents/ --type=ts -B 2 -A 2

Repository: tale-project/tale

Length of output: 5443


🏁 Script executed:

sed -n '180,210p' services/platform/app/features/documents/components/documents-table.tsx

Repository: tale-project/tale

Length of output: 1035


🏁 Script executed:

rg -n 'teamId\s*=' services/platform/app/features/documents/ --type=ts -B 2 -A 2 | head -50

Repository: tale-project/tale

Length of output: 578


🏁 Script executed:

rg -n 'teamId:' services/platform/ --type=ts | head -30

Repository: tale-project/tale

Length of output: 3098


🏁 Script executed:

cat -n services/platform/lib/shared/team_access.ts

Repository: tale-project/tale

Length of output: 136


🏁 Script executed:

cat -n services/platform/convex/lib/team_access.ts

Repository: tale-project/tale

Length of output: 1354


🏁 Script executed:

rg -n 'teamTags|teamIds' services/platform/convex/documents/ --type=ts | head -20

Repository: tale-project/tale

Length of output: 2145


🏁 Script executed:

cat -n services/platform/convex/documents/team_fields.ts

Repository: tale-project/tale

Length of output: 1641


Update team filter to check all assigned teams, not just the first.

The filter currently checks only doc.teamId, which is the first assigned team. This misses documents assigned to multiple teams—if a document is assigned to teams ['sales', 'support'] with teamId='sales', it won't appear when filtering by 'support'.

Update to check teamIds:

Suggested change
    if (selectedTeamIds.length > 0) {
      const teamIdSet = new Set(selectedTeamIds);
      filtered = filtered.filter(
-        (doc) => doc.teamId && teamIdSet.has(doc.teamId),
+        (doc) =>
+          doc.teamIds?.some((id) => teamIdSet.has(id)) ||
+          (doc.teamId && teamIdSet.has(doc.teamId)),
      );
    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/app/features/documents/components/documents-table.tsx`
around lines 192 - 197, The team filter uses doc.teamId (single value) instead
of checking all assigned teams, so documents with multiple team assignments are
missed; update the filter in the documents-table component to check doc.teamIds
(array) against selectedTeamIds by creating a Set from selectedTeamIds and using
Array.prototype.some (or Array.prototype.every depending on desired semantics)
to test if any of doc.teamIds exist in the set (e.g., teamIdSet.has(id) inside
doc.teamIds.some). Ensure you keep the same filtered variable and only apply
this change inside the existing selectedTeamIds.length > 0 block where
doc.teamId is currently referenced.

Comment on lines +47 to 48
const { formatDate, timezoneShort } = useFormatDate();
const { t } = useT('documents');
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Compute the timezone suffix from the row timestamp.

timezoneShort comes from useFormatDate()'s new Date(), so this will always show the current DST abbreviation. A file modified in winter but viewed in summer can end up with a correct local time and the wrong suffix (EST vs EDT). Also, converting lastModified to Date here bypasses formatDate()'s string-specific timezone normalization.

🛠️ Suggested fix
-  const { formatDate, timezoneShort } = useFormatDate();
+  const { formatDate, timezone } = useFormatDate();
...
             {row.original.lastModified
-              ? `${formatDate(new Date(row.original.lastModified), 'long', {
-                  customFormat: 'll LT',
-                })} ${timezoneShort}`
+              ? (() => {
+                  const modifiedAt = new Date(row.original.lastModified);
+                  const timezoneLabel =
+                    new Intl.DateTimeFormat(undefined, {
+                      timeZone: timezone,
+                      timeZoneName: 'short',
+                    })
+                      .formatToParts(modifiedAt)
+                      .find((part) => part.type === 'timeZoneName')?.value ??
+                    timezone;
+
+                  return `${formatDate(row.original.lastModified, 'long', {
+                    customFormat: 'll LT',
+                  })} ${timezoneLabel}`;
+                })()
               : ''}
...
-      timezoneShort,
+      timezone,

Also applies to: 124-127, 158-158

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@services/platform/app/features/documents/components/onedrive-import/onedrive-file-table.tsx`
around lines 47 - 48, The code is using timezoneShort from useFormatDate()
(which uses new Date()) so the suffix reflects "now" rather than the file's
timestamp; instead compute the timezone suffix from the row's timestamp and use
formatDate with the row's lastModified value so both the human date string and
the suffix are normalized to that specific time. Concretely, replace uses of
timezoneShort and Date(row.lastModified) in the OneDrive file table rendering
(references: useFormatDate(), timezoneShort, formatDate(), lastModified) with a
timezone derived from new Date(row.lastModified) (or call a date-aware helper on
that timestamp) and call formatDate(row.lastModified) everywhere (including the
other occurrences around the same component at the noted locations).

Comment on lines +199 to +211
const MAX_VISIBLE = 2;
const names = teamIds.map((id) => teamMap.get(id)).filter(Boolean);
const visible = names.slice(0, MAX_VISIBLE);
const remaining = names.length - MAX_VISIBLE;
return (
<Badge variant="blue" className="text-xs">
{teamName}
</Badge>
<Text as="span" className="text-sm">
{visible.join(', ')}
{remaining > 0 && (
<span className="text-muted-foreground">
{` +${remaining}`}
</span>
)}
</Text>
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider using a type predicate for better type safety.

The filter(Boolean) idiom removes undefined values at runtime, but TypeScript doesn't narrow the resulting type from (string | undefined)[] to string[]. While this works correctly at runtime, adding an explicit type predicate improves type safety and code clarity.

♻️ Suggested improvement
          const MAX_VISIBLE = 2;
-          const names = teamIds.map((id) => teamMap.get(id)).filter(Boolean);
+          const names = teamIds
+            .map((id) => teamMap.get(id))
+            .filter((name): name is string => Boolean(name));
          const visible = names.slice(0, MAX_VISIBLE);
          const remaining = names.length - MAX_VISIBLE;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const MAX_VISIBLE = 2;
const names = teamIds.map((id) => teamMap.get(id)).filter(Boolean);
const visible = names.slice(0, MAX_VISIBLE);
const remaining = names.length - MAX_VISIBLE;
return (
<Badge variant="blue" className="text-xs">
{teamName}
</Badge>
<Text as="span" className="text-sm">
{visible.join(', ')}
{remaining > 0 && (
<span className="text-muted-foreground">
{` +${remaining}`}
</span>
)}
</Text>
const MAX_VISIBLE = 2;
const names = teamIds
.map((id) => teamMap.get(id))
.filter((name): name is string => Boolean(name));
const visible = names.slice(0, MAX_VISIBLE);
const remaining = names.length - MAX_VISIBLE;
return (
<Text as="span" className="text-sm">
{visible.join(', ')}
{remaining > 0 && (
<span className="text-muted-foreground">
{` +${remaining}`}
</span>
)}
</Text>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@services/platform/app/features/documents/hooks/use-documents-table-config.tsx`
around lines 199 - 211, The runtime filter(Boolean) on names (computed from
teamIds.map(id => teamMap.get(id))) doesn't narrow (string | undefined)[] to
string[]; update the filter to use a type predicate such as filter((n): n is
string => n !== undefined) so that names becomes string[] for subsequent
slice/join operations and functions/constants like MAX_VISIBLE, names, teamIds,
and teamMap are handled with correct TypeScript typing.

Comment on lines 68 to +69
teamId: v.optional(v.union(v.string(), v.null())),
teamIds: v.optional(v.array(v.string())),
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Make teamIds the canonical always-present shape.

The new multi-team flow already uses “no selected teams” as a real business state for organization-wide access, so v.optional(v.array(v.string())) adds a second encoding for the same state (undefined vs []). That ambiguity will leak into every consumer and makes it easy to miss the org-wide case or pass undefined into UI that expects an array.

🔧 Suggested change
   teamId: v.optional(v.union(v.string(), v.null())),
-  teamIds: v.optional(v.array(v.string())),
+  teamIds: v.array(v.string()),
   createdBy: v.optional(v.string()),

Then normalize missing legacy data to [] in the document transform, and keep services/platform/types/documents.ts aligned with a non-optional teamIds: string[].

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
teamId: v.optional(v.union(v.string(), v.null())),
teamIds: v.optional(v.array(v.string())),
teamId: v.optional(v.union(v.string(), v.null())),
teamIds: v.array(v.string()),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@services/platform/convex/documents/validators.ts` around lines 68 - 69,
Change the validator and data flow so teamIds is the canonical always-present
array: replace the optional schema v.optional(v.array(v.string())) for the
teamIds field in validators.ts with a required v.array(v.string()), update any
corresponding validators that reference teamId/teamIds, and in the document
transform function (the place that normalizes legacy documents) coerce
missing/undefined teamIds to an empty array ([]). Also update the TypeScript
shape in services/platform/types/documents.ts to declare teamIds: string[] so
the runtime validator, transform, and type definitions are consistent.

- Add shared ZoomPanViewer component (needed by document-preview-image)
- Fix Text component: remove invalid size prop, use label-sm variant
- Fix HStack gap: use 2 instead of unsupported 1.5
- Fix Button variant: use secondary instead of invalid outline
- Fix DetailsSidebar: use teamIds array instead of teamId
- Fix mutation-hooks test: teamId → teamIds
- Rewrite document-preview-routing test mock with beforeAll resolution
- Add type="button" to zoom control buttons in ZoomPanViewer
- Reset loading/error state in DocumentPreviewImage on URL change
- Use teamIds array for team filter in documents table
- Replace div role="group" with native fieldset in team tags dialog
- Add type predicate to filter(Boolean) for type safety
- Always show save/cancel footer in team tags dialog
- Add zoom translation keys to common.imagePreview namespace
Keep teamIds array-based filtering. Update team tags test for always-visible footer.
@Israeltheminer Israeltheminer force-pushed the feat/documents-table-ui-polish branch from 3a352d9 to 1284c43 Compare March 11, 2026 13:06
Use vi.hoisted for lazyPromises to avoid temporal dead zone.
Use getByRole heading to avoid duplicate text match in team tags test.
@Israeltheminer Israeltheminer merged commit 954758a into main Mar 11, 2026
16 checks passed
@Israeltheminer Israeltheminer deleted the feat/documents-table-ui-polish branch March 11, 2026 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant